Improve perf of Utf8Parser.TryParse for int64 and uint64 #52423
Conversation
GrabYourPitchforks
added
area-System.Buffers
tenet-performance
|
Tagging subscribers to this area: @tannergooding, @pgovind, @GrabYourPitchforks Issue DetailsSomewhat inspired by investigating #52314 and seeing a little bit of low-hanging fruit that we can address. Perf results (basically from running this benchmark with some additional inputs): | Method | Toolchain | value | Mean Error | StdDev | Ratio | Some of the benchmarks were going a bit screwy on my machine, so I basically ran them one at a time and concatenated all of the results into a single table. Not sure what was up with my environment. The tl;dr is that this shouldn't have any worse perf than the existing code, and the total codegen ends up being 448 bytes smaller compared to baseline across the two methods.
|
|
|
||
| if ((uint)firstChar == unchecked((uint)('-' - '0'))) | ||
| { | ||
| sign--; // set to -1 |
There was a problem hiding this comment.
| sign--; // set to -1 | |
| sign = -1; |
Make it explicit?
The JIT will emit code like mov rax, 0xffffffffffffffff anyway.
There was a problem hiding this comment.
The JIT emits dec rax for this scenario. I think it doesn't propagate the "if (idx != 0) { FAIL; }" assertion above for some reason.
I can set sign = -1; explicitly, but it does increase the codegen by 7 - 8 bytes. Maybe it's too much of a microoptimization to worry about and the cleaner code is better?
There was a problem hiding this comment.
So I'd keep it as is. Thanks for the info.
| // If sign = -1, this becomes value = (parsedValue ^ -1) - (-1) = ~parsedValue + 1 = -parsedValue | ||
|
|
||
| bytesConsumed = idx; | ||
| value = ((long)parsedValue ^ sign) - sign; |
There was a problem hiding this comment.
🚀
The same could be done to other types too (like int32 above, etc.)?
|
@GrabYourPitchforks Is this ready to merge? |
Ping on this, @GrabYourPitchforks. It would be great to get this in for Preview 7. |
|
I investigated the framework's other parsing routines, but they have a bit more complexity, needing to handle whitespace and trailing nulls. Opened #55541 so we don't lose track of this. |
ghost
locked as resolved and limited conversation to collaborators
Somewhat inspired by investigating #52314 and seeing a little bit of low-hanging fruit that we can address.
Perf results (basically from running this benchmark with some additional inputs):
Some of the benchmarks were going a bit screwy on my machine, so I basically ran them one at a time and concatenated all of the results into a single table. Not sure what was up with my environment.
The tl;dr is that this shouldn't have any worse perf than the existing code, and the total codegen ends up being 448 bytes smaller (on x64) compared to baseline across the two methods.